cpptypes: Adjust the aggregate type of a template specialization type #1

wilsonk · 2015-02-18T08:39:33Z

Adjust the aggregate type of a template specialization type to a templated class declaration, if needed.

… to a templated class declaration, if needed.

Syniurge · 2015-02-18T18:15:27Z

Hey Kelly, I'm glad that you delved deeper into the code and started contributing!

Here however the segfault happened during the mapping of a dependent TemplateSpecializationType and the code assumed that if the type was sugared it was non-dependent which is wrong:

TemplateSpecializationType 0xa1f9b90 'bitset<_Nb>' sugar dependent imported bitset
|-TemplateArgument expr
| `-DeclRefExpr 0xa1f9b68 'size_t':'unsigned long' NonTypeTemplateParm 0xa1e3078 '_Nb' 'size_t':'unsigned long'
`-InjectedClassNameType 0xa1e31c0 'bitset<_Nb>' dependent imported
  `-CXXRecord 0xa1e2f60 'bitset'

The sugared type is the InjectedClassNameType. The solution seems to me to handle InjectedClassNameType like RecordType is. Could you correct the fix so I can add your commit?

You might already know that but GDB is very helpful when trying to figure things out with DMD and Clang, most Clang objects have the dump() method and DMD has toChars() and toPrettyChars(true), which you can call from GDB with "print" and "call".

Then there's wrapping your head around the mess of code and hacks I wrote which is a whole different ball game :)

wilsonk · 2015-02-19T02:15:16Z

Hello Elie,

Thanks for the notice about dump(), and toChars() (I really don’t like DMD so I haven’t been using it, but I will install it now). I use gdb of course. I can see things a little better from the dump()’s. I use llvm’s dump in my other compiler project, but It just outputs the IR and wouldn’t be so useful in this situation…in other words I saw it but didn’t use it because I thought it would just dump IR…duh :)

With the changes below, bitset still compiles and runs. Deque and stack also instantiate now, with the changes you have made and the changes below (only deque needs this fix actually, I believe). I will write some tests for these soon. Vstring/map/array are close now also, I think.

If fromTypeTemplateSpecialization’s body looks correct with the changes below, then let me know and I will push them and update the pull request…otherwise let me know what is wrong and I will fix it again and then push and update (or delete that PR altogether and make a new one).

Type* TypeMapper::FromType::fromTypeTemplateSpecialization(const clang::TemplateSpecializationType* T)

{

auto tqual = fromTemplateName(T->getTemplateName(),

                        T->begin(), T->end());



if (T->isSugared())

{

    auto RT = T->getAs<clang::RecordType>();



    if (RT)

        if (auto subst = tm.trySubstitute(RT->getDecl())) // HACK for correctTiargs                                                     

            return subst;



    if (RT && !RT->isDependentType())

    {

        RootObject *o;

        if (tqual->idents.empty())

            o = static_cast<TypeInstance*>(tqual)->tempinst;

        else

            o = tqual->idents.back();

        auto ti = (cpp::TemplateInstance*)o;



        ti->Instances[ti->name] = RT->getDecl();

        ti->completeInst();

    }



    if (T->isTypeAlias())

    {

        auto DNT = T->getAs<clang::DependentNameType>();

        if (DNT)

            return fromTypeDependentName(DNT);

   }

    else

    {

        if (!RT)

        {

            auto ICNT = T->getAs<clang::InjectedClassNameType>();

            return adjustAggregateType(tqual, ICNT->getDecl());

        }

        else

            return adjustAggregateType(tqual, RT->getDecl());

    }

}

return adjustAggregateType(tqual);

}

Thanks,

Kelly

From: Elie Morisse [mailto:[email protected]]
Sent: Wednesday, February 18, 2015 11:15 AM
To: Syniurge/Calypso
Cc: wilsonk
Subject: Re: [Calypso] cpptypes: Adjust the aggregate type of a template specialization type (#1)

Hey Kelly, I'm glad that you delved deeper into the code and started contributing!

Here however the segfault happened during the mapping of a dependent TemplateSpecializationType and the code assumed that if the type was sugared it was non-dependent which is wrong:

TemplateSpecializationType 0xa1f9b90 'bitset<_Nb>' sugar dependent imported bitset
|-TemplateArgument expr
| -DeclRefExpr 0xa1f9b68 'size_t':'unsigned long' NonTypeTemplateParm 0xa1e3078 '_Nb' 'size_t':'unsigned long' -InjectedClassNameType 0xa1e31c0 'bitset<_Nb>' dependent imported
`-CXXRecord 0xa1e2f60 'bitset'

The sugared type is the InjectedClassNameType. The solution seems to me to handle InjectedClassNameType like RecordType is. Could you correct the fix so I can add your commit?

You might already know that but GDB is very helpful when trying to figure things out with DMD and Clang, most Clang objects have the dump() method and DMD has toChars() and toPrettyChars(true), which you can call from GDB with "print" and "call".

Then there's wrapping your head around the mess of code and hacks I wrote which is a whole different ball game :)

—
Reply to this email directly or view it on GitHub #1 (comment) .

Syniurge · 2015-02-19T03:00:09Z

Looks good, just one thing: castAs<> would be better than getAs<> for DNT and ICNT.
Also what does the DNT part fix?

If you have more test cases for STL types could you do PR(s) for them? I'm working on std::map and don't have any other STL examples beyond that.

wilsonk · 2015-02-19T04:23:45Z

Hello Elie,

I will get the STL examples together here and make a PR for them soon. I changed DNT and ICNT to castAs<> but things don’t work for DNT with a cast, it needs to stay getAs<>.

The example that was triggering this was modmap’ing for vstring.d like :

modmap (C++) "vector";

modmap (C++) "string";

import (C++) std.vector;

.

TemplateSpecializationType 0x97b0b30 '_RequireInputIter<_InputIterator>' sugar dependent imported alias _RequireInputIter

|-TemplateArgument type '_InputIterator'

|-DependentNameType 0x97b0b00 'typename enable_if<is_convertible<typename iterator_traits::iterator_category, struct input_iterator_tag>::value>::type' dependent imported

`-DependentNameType 0x97b0ad0 'typename enable_if<is_convertible<typename iterator_traits::iterator_category, struct input_iterator_tag>::value, void>::type' dependent imported

This happened when I just changed the bad code from the PR out with:

            auto ICNT = T->getAs<clang::InjectedClassNameType>();

            return adjustAggregateType(tqual, ICNT->getDecl());

The error seemed to be fixed if I checked for an alias first and then returned the dependentNameType…maybe that is still wrong? Seems strange that it is only triggered when both headers are imported. ?? Geez I just checked some more and this is only triggered when –cpp-args –std=c++11 is used to compile, so I guess it is a c++11 feature.

Thanks,

Kelly

From: Elie Morisse [mailto:[email protected]]
Sent: Wednesday, February 18, 2015 8:00 PM
To: Syniurge/Calypso
Cc: wilsonk
Subject: Re: [Calypso] cpptypes: Adjust the aggregate type of a template specialization type (#1)

Looks good, just one thing: castAs<> would be better than getAs<> for DNT and ICNT.
Also what does the DNT part fix?

If you have more test cases for STL types could you do PR(s) for them? I'm working on std::map and don't have any other STL examples beyond that.

—
Reply to this email directly or view it on GitHub #1 (comment) .

wilsonk · 2015-02-19T04:31:50Z

Helle Elie,

I tested a little more and getAs or castAs doesn’t make a difference actually. Sorry.

Thanks,

Kelly

From: Elie Morisse [mailto:[email protected]]
Sent: Wednesday, February 18, 2015 8:00 PM
To: Syniurge/Calypso
Cc: wilsonk
Subject: Re: [Calypso] cpptypes: Adjust the aggregate type of a template specialization type (#1)

Looks good, just one thing: castAs<> would be better than getAs<> for DNT and ICNT.
Also what does the DNT part fix?

If you have more test cases for STL types could you do PR(s) for them? I'm working on std::map and don't have any other STL examples beyond that.

—
Reply to this email directly or view it on GitHub #1 (comment) .

Syniurge · 2015-02-19T16:21:40Z

Do std::string and std::vector instantiate when C++11 is enabled? Last time I checked there were identifier lookup issues with std::vector IIRC, and complicated ones to solve so I kinda put C++11 aside.

( What happens is that whenever Clang's template argument deduction is involved the SubstTemplateTypeParmType sugar is lost so Calypso ends with identifiers the D template instance doesn't know (because the scope of a D template instantiation is much tighter than in C++: http://dlang.org/template.html "Instantiation Scope"), and the C++11 version of STL types use arg deduction in more places than in pre-C++0x. )

I'll have a look this evening. Could you already request pulling the ICNT part? I checked yesterday and it works.

… InjectedClassNameTypes

wilsonk · 2015-02-19T19:31:10Z

Hello Elie,

Sorry, I pushed that ICNT code to the pull request quickly (I thought I did it last night actually).

Something is going on with c++11 and std::vector/std::string, because the DNT error I posted is only triggered when c++11 is used, and you modmap + import vector AND string. The main function in my example code is just empty and I still get this error. No triggering of the DNT error when c++11 isn’t used.

I can make a separate PR for DNT, then it can be removed or searched easily later, if you like. Or we can just leave it out and not compile with c++11 :)

Thanks,

Kelly

From: Elie Morisse [mailto:[email protected]]
Sent: Thursday, February 19, 2015 9:22 AM
To: Syniurge/Calypso
Cc: wilsonk
Subject: Re: [Calypso] cpptypes: Adjust the aggregate type of a template specialization type (#1)

Do std::string and std::vector instantiate when C++11 is enabled? Last time I checked there were identifier lookup issues with std::vector IIRC, and complicated ones to solve so I kinda put C++11 aside.

( What happens is that whenever Clang's template argument deduction is involved the SubstTemplateTypeParmType sugar is lost so Calypso ends with identifiers the D template instance doesn't know (because the scope of a D template instantiation is much tighter than in C++: http://dlang.org/template.html "Instantiation Scope"), and the C++11 version of STL types use arg deduction in more places than in pre-C++0x. )

I'll have a look this evening. Could you already request pulling the ICNT part? I checked yesterday and it works.

—
Reply to this email directly or view it on GitHub #1 (comment) .

Syniurge · 2015-02-19T22:33:53Z

Almost there! :) The second "return adjustAggregateType(tqual, RT->getDecl());" added by the PR isn't correct or at least it's not supposed to work like that. If it's a type alias the alias already makes it a TypeValueof, so no need to add another one on top of it, I tried to make TypeValueof as ephemereal as possible.

wilsonk · 2015-02-19T23:02:21Z

Ugh, I should have reviewed the first commit and seen that…I thought it was there originally. Sometimes I hate git from the command line. Three commits for two lines on code changes …. Weeeee!

Hopefully that’s finished :)

Thanks,

Kelly

From: Elie Morisse [mailto:[email protected]]
Sent: Thursday, February 19, 2015 3:34 PM
To: Syniurge/Calypso
Cc: wilsonk
Subject: Re: [Calypso] cpptypes: Adjust the aggregate type of a template specialization type (#1)

Almost there! :) The second "return adjustAggregateType(tqual, RT->getDecl());" added by the PR isn't correct or at least it's not supposed to work like that. If it's a type alias the alias already makes it a TypeValueof, so no need to add another one on top of it, I tried to make TypeValueof as ephemereal as possible.

—
Reply to this email directly or view it on GitHub #1 (comment) .

TemplateSpecializationType might be sugar for InjectedClassNameType too.

wilsonk · 2015-02-20T06:04:46Z

Hello Elie,

Std::list now compiles with your last changes :) There are a few errors at semantic3 for things like this:

/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_iterator.h:232:41: error: invalid operands to binary expression ('const struct std::_List_const_iterator' and 'difference_type' (aka 'long'))

  { return reverse_iterator(current - __n); }

but the resulting binary can instantiate a minimal example. I will write other tests for lists and see how it goes.

I have a deque test file put together now also. I will work on the other files and push the changes to a pull request. Do you mind if a put each one in a new directory? Maybe make an auto-test runner so that testing is easier?

Thanks,

Kelly

From: Elie Morisse [mailto:[email protected]]
Sent: Thursday, February 19, 2015 4:18 PM
To: Syniurge/Calypso
Cc: wilsonk
Subject: Re: [Calypso] cpptypes: Adjust the aggregate type of a template specialization type (#1)

Merged #1 #1 .

—
Reply to this email directly or view it on GitHub #1 (comment) .

A major source of headaches and probably the source #1 of mapping failures for sophisticated C++ libraries has been the restrictions of D template instantiation scope. Those restrictions are totally necessary and work great in D, and Clang adding "sugar" to its types made it possible to comply with them... most of the time. One particular wall was hit while trying to map the MSVC standard library. For type_traits's conjunction<_Is_character<char>, _Is_character<char>>, the base class in Clang's AST is: TemplateSpecializationType 0xdfaed30 '_Conjunction<struct std::_Is_character<char>, struct std::_Is_character<char> >' sugar _Conjunction | -TemplateArgument type 'struct std::_Is_character<char>':'struct std::_Is_character<char>' | SubstTemplateTypeParmType 0xdda9f20 'struct std::_Is_character<char>' sugar | -TemplateTypeParmType 0xa7ce0f0 '_Traits' dependent contains_unexpanded_pack depth 0 index 0 pack | `-TemplateTypeParm 0xa7ce0c0 '_Traits' `-RecordType 0xa9079d0 'struct std::_Is_character<char>' `-ClassTemplateSpecialization 0xa907930 '_Is_character' | -TemplateArgument type 'struct std::_Is_character<char>' : 'struct std::_Is_character<char>' | SubstTemplateTypeParmType 0xdda9f20 'struct std::_Is_character<char>' sugar (...same as above....) `-RecordType 0xdfaed10 'struct std::_Conjunction<struct std::_Is_character<char>, struct std::_Is_character<char> >' `-ClassTemplateSpecialization 0xdfaec78 '_Conjunction' This is one of those rare cases where Clang has lost pack information and no simple/good heuristic to guess when two or more arguments come from the same pack appears to be possible. Hence either Clang needs to be modified to preserve pack info, or a complicated check/heuristic is needed, or we've got to stop trying to stick to DMD's way. The third option was chosen because there's actually no good reason to comply with those restrictions. Reflection doesn't get improved, and while instantiation from C++ modules, every referenced symbol is from C++ modules. So from now on: - Every type gets desugared before being mapped in non-dependent contexts. - C++ modules now have access to the "C++ global namespace" during name lookups, through a special type of global import. This allows to discard the C++ symbol collector and substitution complicated hack when mapping template arguments deduced by Sema.

…s/structs. It works in most cases but disambiguating non-member calls from member calls requires a lot more code, so most of this will be replaced by switching to Clang for overload candidate selection.

cpptypes: Adjust the aggregate type of a template specialization type…

244b4a7

… to a templated class declaration, if needed.

cpptypes: Fix that last commit since it was really just an issue with…

9a17b93

… InjectedClassNameTypes

cpptypes: remove extraneous return value from first commit

1b10a80

Syniurge added a commit that referenced this pull request Feb 19, 2015

Merge pull request #1 from wilsonk/templatedDecl

bab8eef

TemplateSpecializationType might be sugar for InjectedClassNameType too.

Syniurge merged commit bab8eef into Syniurge:master Feb 19, 2015

This was referenced Apr 21, 2015

Change StringLiteral mapping for wide chars and add a test case. #11

Closed

Dlib matrix.h modmap error #13

Closed

wilsonk mentioned this pull request May 9, 2015

Qt5 demo #17

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cpptypes: Adjust the aggregate type of a template specialization type #1

cpptypes: Adjust the aggregate type of a template specialization type #1

wilsonk commented Feb 18, 2015

Syniurge commented Feb 18, 2015

wilsonk commented Feb 19, 2015

Syniurge commented Feb 19, 2015

wilsonk commented Feb 19, 2015

wilsonk commented Feb 19, 2015

Syniurge commented Feb 19, 2015

wilsonk commented Feb 19, 2015

Syniurge commented Feb 19, 2015

wilsonk commented Feb 19, 2015

wilsonk commented Feb 20, 2015

cpptypes: Adjust the aggregate type of a template specialization type #1

cpptypes: Adjust the aggregate type of a template specialization type #1

Conversation

wilsonk commented Feb 18, 2015

Syniurge commented Feb 18, 2015

wilsonk commented Feb 19, 2015

Syniurge commented Feb 19, 2015

wilsonk commented Feb 19, 2015

wilsonk commented Feb 19, 2015

Syniurge commented Feb 19, 2015

wilsonk commented Feb 19, 2015

Syniurge commented Feb 19, 2015

wilsonk commented Feb 19, 2015

wilsonk commented Feb 20, 2015